Skip to content
This repository was archived by the owner on Jan 27, 2026. It is now read-only.

Add clampedNumberValidator for input fields#157

Merged
islemaster merged 1 commit intomasterfrom
clamped-number-validator
Nov 16, 2018
Merged

Add clampedNumberValidator for input fields#157
islemaster merged 1 commit intomasterfrom
clamped-number-validator

Conversation

@islemaster
Copy link
Contributor

Part of the solution for code-dot-org/dance-party#321

Allows us to have a freeform text input in a block that can be constrained to accepting numeric input within a specified range. The range can have a lower bound, and upper bound, or both. The corresponding input element will be given the "number" type by default so the student cannot even type non-numeric characters into the field. Invalid or blank input results in zero, unless zero is outside the given bounds in which case the zero is clamped to the nearest boundary.

Part of the solution for code-dot-org/dance-party#321

Allows us to have a freeform text input in a block that can be constrained to accepting numeric input within a specified range.  The range can have a lower bound, and upper bound, or both.  The corresponding input element will be given the "number" type by default so the student cannot even type non-numeric characters into the field.  Invalid or blank input results in zero, unless zero is outside the given bounds in which case the zero is clamped to the nearest boundary.
@islemaster islemaster requested a review from Hamms November 16, 2018 01:38
@codecov-io
Copy link

Codecov Report

Merging #157 into master will decrease coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #157      +/-   ##
==========================================
- Coverage   55.55%   55.48%   -0.07%     
==========================================
  Files          96       96              
  Lines       11476    11486      +10     
==========================================
- Hits         6375     6373       -2     
- Misses       5101     5113      +12
Impacted Files Coverage Δ
core/ui/fields/field_textinput.js 65.88% <100%> (+2.36%) ⬆️
blocks/variables.js 71.92% <0%> (-3.51%) ⬇️
generators/javascript.js 93.05% <0%> (-2.78%) ⬇️
generators/javascript/text.js 11.67% <0%> (-1.46%) ⬇️
core/code_generation/generator.js 81.91% <0%> (-1.25%) ⬇️
core/utils/xml.js 85.15% <0%> (-1.18%) ⬇️
core/utils/variables.js 65.71% <0%> (-0.72%) ⬇️
core/ui/block_space/block_space.js 57.48% <0%> (-0.22%) ⬇️
core/ui/fields/field_variable.js 54.68% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bed1114...3d001a5. Read the comment docs.

islemaster added a commit to code-dot-org/code-dot-org that referenced this pull request Nov 16, 2018
Depends on code-dot-org/blockly#157

A custom block can give any field input a type of `"ClampedNumber(min,max)"` where `min` and `max` are optional and define bounds for acceptable number input to the field.
@islemaster
Copy link
Contributor Author

See it in action on code-dot-org/code-dot-org#26124

Copy link
Contributor

@Hamms Hamms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for missing this yesterday! One comment, otherwise LGTM!

Also: those tests 😍

if (this.changeHandler_ === Blockly.FieldTextInput.numberValidator) {
if (
(this.changeHandler_ === Blockly.FieldTextInput.numberValidator) ||
(this.changeHandler_ && this.changeHandler_.validatorType === 'clampedNumberValidator'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love that we're introducing inconsistency with the way you identify a changeHandler; can we create an issue to track improving this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 That's a good idea. I messed around with identifying these by function name or prototype instead of by identity, but decided in the end it'd be too clever by half and I should just add a property. I'd love to revisit this and make it make sense.

#158

@islemaster islemaster merged commit 967bad1 into master Nov 16, 2018
@islemaster islemaster deleted the clamped-number-validator branch November 16, 2018 18:50
islemaster added a commit to code-dot-org/code-dot-org that referenced this pull request Nov 16, 2018
Picks up code-dot-org/blockly#157 which is essential for this change, as well as code-dot-org/blockly#156 which we wanted to ship anyway, and code-dot-org/blockly#152 from a contributor.
islemaster added a commit to code-dot-org/code-dot-org that referenced this pull request Nov 19, 2018
Depends on code-dot-org/blockly#157

A custom block can give any field input a type of `"ClampedNumber(min,max)"` where `min` and `max` are optional and define bounds for acceptable number input to the field.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants